-
Notifications
You must be signed in to change notification settings - Fork 262
Chore(evm) add combine pricefeed #2665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
7 Skipped Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is much better than last time. couple comments here. I suggest we sit down in person and work through them -- should be pretty quick
|
||
int32 factoredExpo = combinedExpo - targetExpo; | ||
|
||
if (factoredExpo > 77 || factoredExpo < -77) revert PythErrors.ExponentOverflow(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't you put all of this exponent stuff into the Math.mulDiv call ^ and be done?
like you're trying to compute z
in:
z * 10^c = (x * 10^a) / (y * 10^b)
which you can rewrite to:
z = (x * 10^(a - (b + c))) / y
so can't you do like:
uint256 result = Math.mulDiv(uint64(price1), 10 ** (expo1 - (expo2 + targetExpo)), uint64(price2));
and then you're done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Math.mulDiv is a good find -- that's a very useful primitive for this kind of arithmetic)
@@ -33,4 +33,38 @@ library PythUtils { | |||
10 ** uint32(priceDecimals - targetDecimals); | |||
} | |||
} | |||
|
|||
/// @notice Combines two prices to get a cross-rate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing i would document in particular is: under what range of exponents is this function guaranteed not to overflow for any set of prices. That's the safe range to use this function in.
@@ -33,4 +33,38 @@ library PythUtils { | |||
10 ** uint32(priceDecimals - targetDecimals); | |||
} | |||
} | |||
|
|||
/// @notice Combines two prices to get a cross-rate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also document that you get the floor of the result in the target exponent, so small numbers can round to 0.
Summary
Rationale
How has this been tested?